Skip to content

fix(metadata-protocol): classify a failed index build from the ERROR, not its message (#6699) - #6845

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-6699-classify-index-failure-error-channel
Aug 9, 2026
Merged

fix(metadata-protocol): classify a failed index build from the ERROR, not its message (#6699)#6845
os-zhuang merged 1 commit into
mainfrom
claude/issue-6699-classify-index-failure-error-channel

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #6699

What

classifyIndexFailure is the fifth private unique-violation vocabulary — the one #6250's inventory missed, because it lives in a package none of the other four touched. Its first arm now delegates to @objectstack/types' isUniqueViolationError (#6250 / PR #6541), and probeThenReplaceIndex passes it the caught error object instead of err.message.

The threading is the substance, not the delegation. A string-only swap compiles unchanged and keeps the defect: the code / errno / cause channels are the entire reason the shared predicate exists.

Premise check — the card's file anchor has moved (the finding itself holds)

The issue body and the triage comment both place classifyIndexFailure at packages/metadata-protocol/src/migrations/view-definition-active-index.ts:279. On origin/main @ 220536377 it is not there: #6418 moved it to migrations/partial-index-probe.ts:102 when ensureOverlayIndex adopted the probe-first order, and view-definition-active-index.ts:196 now only re-exports it. Everything the finding asserts is still true at the new address — still message: string only, still its own private regex, and the shared predicate still has zero consumers in this package. The fix therefore landed in partial-index-probe.ts.

Two consequences worth recording:

Measured, on origin/main before the change

Old (message-only) versus new (object), same inputs:

case old new
SQLite UNIQUE constraint failed: ... conflict conflict
MySQL Duplicate entry 'a' for key 'i' conflict conflict
Postgres duplicate key value violates unique constraint conflict conflict
near "WHERE": syntax error unsupported unsupported
Functional index on a column is not supported unsupported unsupported
disk I/O error failed failed
code: 'SQLITE_CONSTRAINT_UNIQUE', message insert failed failed conflict
code: 'ER_DUP_ENTRY', errno: 1062, message Write failed failed conflict
errno: 1062 alone failed conflict
code: '23505', message insert failed failed conflict
cause.code: '23505' behind pool query failed failed conflict
code: '23505' + dialect wording in the message unsupported conflict

No message-channel verdict moves: the shared predicate's message limb is a superset of the four substrings this module carried, across all three shipped dialects. The last row is the arm order surviving the widening — a real data conflict that would otherwise read as "this database cannot build this index".

The two load-bearing constraints, preserved

  1. Arm order unchanged. Duplicate/unique is judged BEFORE dialect wording, because MySQL's duplicate error mentions the key and some drivers wrap both facts in one string. The doc comment keeps that rationale and now also states why the first arm is no longer a local regex.
  2. The dialect arm is untouched. isUniqueViolationError answers the first arm only and has no opinion about dialect support, so the unsupported vocabulary stays this module's own. It reads the message channel via a new private indexFailureText, which prefers message and falls back to String() — a bare string resolves to itself, so prose callers are judged exactly as before.

packages/types was consumed strictly read-only; its contract was sufficient as-is.

Signature

classifyIndexFailure(message: string) becomes classifyIndexFailure(error: unknown). A widening, so every existing string call still compiles and is judged identically. detail in ProbeThenReplaceOutcome is unchanged — still the driver's own prose, for the operator.

Reverse verification — direction predicted before running, in two stages

Predicted and confirmed. Split deliberately, because the two halves of the change are separately reversible and the unit tests can only see one of them.

Stage A — revert only the call-site threading, keep the new classifier. Predicted: exactly one red, the threading pin. Result:

× the probe hands the ERROR to the classifier, not its message (#6699)
  AssertionError: expected 'failed' to be 'conflict'
Test Files  1 failed | 62 passed (63)
Tests  1 failed | 754 passed (755)

This is why that test exists: every classifier-level assertion stays green when the caller unwraps the message first.

Stage B — revert the whole source file to origin/main, keep the new tests. Predicted: all object-channel pins red, all prose pins green. Result — 8 red, all object-channel, 747 green:

× reads a conflict off a SQLite extended result code ...   expected 'failed' to be 'conflict'
× reads a conflict off mysql2's symbolic name ...          expected 'failed' to be 'conflict'
× reads a conflict off a bare MySQL errno ...              expected 'failed' to be 'conflict'
× reads a conflict off a Postgres SQLSTATE ...             expected 'failed' to be 'conflict'
× follows a pooled wrapper down to the cause (#6699)       expected 'failed' to be 'conflict'
× keeps the data verdict ahead of the dialect verdict ...  expected 'unsupported' to be 'conflict'
× the probe hands the ERROR to the classifier ...          expected 'failed' to be 'conflict'
× classifies duplicate-row wording as a conflict ...       expected 'failed' to be 'conflict'
Test Files  2 failed | 61 passed (63)
Tests  8 failed | 747 passed (755)

Every prose pin stayed green under the reverted source, which is the no-regression half. One test is deliberately green in both directions and is reported as such rather than dressed up: a dialect refusal carrying its own code is still 'unsupported' pins that widening the input did not blind the second arm — its job is to stay green, so it can never be evidence the fix works.

Tests

New pins in partial-index-probe.test.ts (the classifier's home):

  • four it.each cases, one per channel, each asserting the object is conflict and that the same prose alone is failed — so the case proves the verdict cannot have come from the message channel;
  • the cause walk behind a pooled wrapper;
  • arm order on the object channel, plus the single-string form unchanged since [metadata-protocol] ensureOverlayIndex 先 DROP 后 CREATE:partial 索引建失败时 sys_metadata 会静默地失去覆盖层唯一约束 #6418;
  • a MySQL parse error carrying ER_PARSE_ERROR / 1064 still unsupported, and an I/O error carrying SQLITE_IOERR still failed — the widening must not turn the second arm blind;
  • the end-to-end threading pin through probeThenReplaceIndex against real SQLite, which also asserts detail is still the driver's prose and the previous index is untouched.

One assertion added to view-definition-active-index.test.ts's existing seam test, so the public @objectstack/metadata-protocol re-export is pinned to carry the object-channel behaviour too.

pnpm --filter @objectstack/metadata-protocol test
Test Files  63 passed (63)
Tests  755 passed (755)

tsc --noEmit -p packages/metadata-protocol — 0 errors in the changed files; the package total is 63, exactly its recorded DEBT ledger entry, so the ratchet does not move. eslint on the changed paths clean. check:nul-bytes, check:durability-log-level, check:adr-anchors, check:doc-authoring, check:empty-changeset, check:startup-registry-verdict, check:error-code-casing, check:type-check-coverage all pass. check:type-check-debt refuses to run without a full build closure (its own #6376 guard, an environment precondition rather than a result) and is left to CI, which builds the closure first.

Changeset: @objectstack/metadata-protocol patch.


Generated by Claude Code

… not its message (#6699)

`classifyIndexFailure` carried a fifth private unique-violation vocabulary —
the one #6250's inventory missed, because it lives in a package none of the
other four touched — and answered from the message channel only.

The first arm now delegates to `@objectstack/types`' `isUniqueViolationError`,
and `probeThenReplaceIndex` passes it the caught ERROR OBJECT rather than
`err.message`. A string-only swap would have compiled unchanged and kept the
defect: the `code` / `errno` / `cause` channels are the point of the shared
predicate.

A conflict reported on `code`/`errno` with unhelpful prose (SQLite's
`SQLITE_CONSTRAINT_UNIQUE`, MySQL's `ER_DUP_ENTRY`/1062, Postgres' `23505`, or
one step down `cause`) was classified `failed`; it is now `conflict`, which is
the verdict that produces ADR-0120 D4's report. Every message-channel verdict
is unchanged.

Preserved deliberately: the arm order (duplicate wording judged BEFORE dialect
wording, since MySQL's duplicate error mentions the key and some drivers wrap
both facts in one string), and the dialect arm as this module's own
message-based vocabulary — the shared predicate answers the first arm only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 9, 2026 12:16am

Request Review

@github-actions github-actions Bot added the size/m label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

4 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)
  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 9, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 9, 2026 00:29
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 9, 2026
Merged via the queue into main with commit 53aeb02 Aug 9, 2026
25 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-6699-classify-index-failure-error-channel branch August 9, 2026 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] classifyIndexFailure is a FIFTH private unique-violation vocabulary, missed by #6250's inventory — and it reads only the message channel

2 participants